Repository navigation
Conversation
There was a problem hiding this comment.
Code Review
This pull request removes the restriction and associated tests that rejected the WRITE_APPEND write disposition when creating an Arrow query stream. Specifically, the check for WRITE_APPEND in createQueryArrowStream_ has been removed, along with the corresponding unit and system tests. There are no review comments to address.
|
Can you add a couple of tests confirming that |
977c17a to
4d56cd1
Compare
4d56cd1 to
8f3395e
Compare
Remove the guard that rejected WRITE_APPEND. With explicit destination tables, the stream reads the destination through a read session, so a WRITE_APPEND query returns the existing rows plus the appended ones. This matches query(), which also reads back the whole destination table. Replace the rejection tests with a unit test that WRITE_APPEND reaches the jobs.insert configuration, and a system test that WRITE_APPEND returns existing and new rows while WRITE_EMPTY fails on a non-empty table and leaves it unchanged.
|
@shivanee-p Added. The new system test checks that |
8f3395e to
1428bca
Compare
…w-allow-write-append
With WRITE_APPEND the destination table also holds rows from before the query. Say so in the createQueryArrowStream JSDoc, as query() behaves the same, and limit the read-session comment about row order to a table the query just wrote.
Context & Problem
createQueryArrowStreamrejectedwriteDisposition: 'WRITE_APPEND'. Per the design doc discussion on write dispositions, Arrow should matchquery(), which reads back the whole destination table.Summary of Changes
WRITE_APPENDguard. With an explicit destination, the stream reads the table through a read session, so it returns the rows already in the table plus the appended ones. The JSDoc says so.WRITE_APPENDreachesconfiguration.queryonjobs.insert) and a system test (WRITE_APPENDreturns the old and new rows;WRITE_EMPTYon a non-empty table fails and leaves it unchanged).Verification
Unit tests pass (594), and the Arrow system tests pass against the real API (25 passing, 1 pending: the
TIMESTAMP(12)test needsBIGQUERY_PICOSECOND_SUPPORT).